Clippy seems to have found some new things to be upset about in 1.43.

When looking further into how to divide the the hierarchy I realized that there are other ranges that are required to keep track of. E.g., in order to present the ports/generics as a sub-hierarchy I need to track the range between port/generic and the closing ;. Similar goes for dividing the declarative region and the statements.

I think that a solution would be to change the source_range field to a new struct:

pub struct SourceRanges {
    pub unit: SrcPos, // Position of unit start token (e.g. entity) to semicolon at the end of design unit
    pub generic_clause: Option<SrcPos>, // Range from 'generic' to ';' (inclusive) 
    pub port_clause: Option<SrcPos>, // Range from 'port' to ';' (inclusive)
    pub begin: Option<SrcPos>, // Range of 'begin' token
    pub end: SrcPos, // Range of 'end' token
}

This would allow the creation of the following hierarchy (each level fold-able at least in VSCode)

designunitname
    context clause (start of first context item to start of SourceRanges::unit)
        <context items>
    generics
        <interface declarations>
    ports
        <interface declarations>
    declarations
        <declarations>
    statements (SourceRanges::begin to SourceRanges::end)
        <statements>

The alternative would be to to either change the port/generic clauses to WithPos<Option<Vec<InterfaceDeclaration>>> or create a new struct

struct InterfaceList {
    items: Vec<InterfaceDeclaration>,
    source_range: SrcPos
}

and similar for vectors of declarations/statements.

I like the last option the best and the first option the least. It is better to attach the range close to where it associated and tie it together with a struct like the last option. The first option seems wierd to collect all ranges by themselves in a struct even though they are associated with different parts of the AST.

Another alternative would be to strictly store the ranges of the tokens or the tokens themselves dividing the different parts , e.g. for an entity, is, begin, end, and ; and let downstream functions decide how they want to generate the ranges (or make other use of the token locations). The port list would get the new InterfaceList struct which would store the port/generic/parameter token as well as the closing ;.

A benefit of storing the token (or a subset of the token since the value and next_state are of little use) would also be that downstream function get access to the comments related to the token. It would not capture all comments, but enough to enable "documentation" comments in e.g. Hover.

This way I would claim that the information is kept with the appropriate AST object. I also think that it would introduce less logic to the parser and defer range calculation and combining of ranges to when the range is actually needed.

An updated Entity declaration would then look something like:

pub struct KeyWordToken { // Probably needs a better name. Used for keywords and separators like `;`
    pub kind: Kind,
    pub pos: SrcPos,
    pub comments: Option<Box<TokenComments>>,
}

impl From<Token> for KeyWordToken {
    ...
}

struct InterfaceList {
    items: Vec<InterfaceDeclaration>,

    // Tokens
    source_range: SrcPos,
}

pub struct EntityDeclaration {
    pub context_clause: ContextClause,
    pub ident: Ident,
    pub generic_clause: Option<InterfaceList>,
    pub port_clause: Option<InterfaceList>,
    pub decl: DeclarativeRegion,
    pub statements: ConcurrentStatementPart,

    // Tokens
    pub entity_token: KeyWordToken,
    pub is_token: KeyWordToken,
    pub begin_token: Option<KeyWordToken>,
    pub end_token: KeyWordToken,
    pub semi_token: KeyWordToken
}

I think your latest idea is good.

Updated select AST objects with tokens and the hierarchy down to declarations/statements.

The range for declarations and statements is just the range of the label or identifier, tests are not fully implemented.

Yes as you say, a bit of testing is missing. Otherwise looks ok.

Any update on this?

It should be close to complete and I can free some time to finalize it.

That would be great.

After looking at this again I think it would be best if the AST contained higher level information rather than tokens. I like the idea of attaching a source range directly to the InterfaceLists and DeclarativeRegion using something like a WithRange<T> wrapper struct. This also makes it easier for the parser to recover from a missing semi colon while still producing output for analysis. If the range is based on actually storing the semi colon in the struct it cannot be expressed if the semi colon is missing.

I would like to include the following in the outline:

• Context clause (I think that in the outline I did it was folded into the design unit but still needs a range)
• Primary and Secondary design units
• Declarations, both in port/generic/parameter lists and in the declarative region of the design units.
• Concurrent statements (concurrent signal assignments would be folded into a group).

What would the difference be between a WithPos and a WithRange? I like the concept of being able to use the range of the wrapped object to find the source. Semantically, the name would perhaps not be indicative of it describing a range but functionally it would be identical.

To clarify the basic principle:

• Structs for AST objects (such as EntityDeclaration) should be extended with a SrcPos field
• Collections of AST objects using "standard" types such as vector should be wrapped in a new WithRange or the existing WithPos.

You are right that WithPos is already there and a WithRange is redundant. A SrcPos is already a range.

Closing in favor of PR #114